Attention is currently required from: iedemam, neels, laforge, pespin, dexter.
fixeria 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 5: Code-Review-1
(2 comments)
File src/osmo-bsc/bts_vty.c:
https://gerrit.osmocom.org/c/osmo-bsc/+/28276/comment/f34745a7_1e9413ef
PS5, Line 568: chan_alloc_allow
Why are you removing this part?
It's still a channel allocator specific option, right?
https://gerrit.osmocom.org/c/osmo-bsc/+/28276/comment/84e3e321_db602deb
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 'never|voice|always'). You should add an alias for backwards-compatibility, `grep ALIAS_DEPRECATED` for examples.
--
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: 5
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: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: dexter <pmaier(a)sysmocom.de>
Gerrit-Comment-Date: Wed, 15 Jun 2022 09:14:10 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: laforge.
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 don't like mixing conventions in a single project (here: libosmocore), sorry :/
up to what level does this hold?
I assume I have to move the per-subdir Makefile.am that we have in all other osmo.gits to the single toplevel Makefile.am scheme that is in libosmocore.git.
I can rename tests/libosmo-gtlv to tests/gtlv.
Do I have to remove the dash from "libosmo-gtlv"? It would match libosmogsm, libosmovty and the others in libosmocore.git, but it would not match libosmo-pfcp, libosmo-sccp, libosmo-abis, etc. I'd have to also remove the dash in the places that use libosmo-gtlv. The dash was fine in osmo-upf.git, I'd like to keep it.
All of the above is functionally invisible gruntwork.
Can I still change my mind and rather move libosmo-gtlv to libosmo-pfcp.git?
Or, what about a separate libosmo-gtlv.git?
--
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-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Comment-Date: Tue, 14 Jun 2022 22:10:48 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: laforge <laforge(a)osmocom.org>
Gerrit-MessageType: comment
Attention is currently required from: neels.
Hello Jenkins Builder,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/libosmocore/+/28286
to look at the new patch set (#2).
Change subject: add libosmo-gtlv, moved from osmo-upf.git
......................................................................
add libosmo-gtlv, moved from osmo-upf.git
libosmo-gtlv does not strictly conform to the libosmocore Makefile.am
structure, because it was first added in osmo-upf.git, and because we
use other Makefile.am conventions outside of libosmocore.git:
- use of SUBDIRS instead of single top level Makefile.am
- name of libosmo-gtlv with a dash, other libs here omit a dash.
- name of src dirs being "libosmo-gtlv" instead of just "gtlv".
I hope that code review agrees with this choice of rather keeping the
code conforming to our conventions that we established later, and not
strictly changing to follow "legacy" conventions in libosmocore.git.
Pasting initial libosmo-gtlv commit logs from osmo-upf.git:
(1) TLV skeleton traversal:
An all new TLV parser supporting:
- Any size of T and L (determined by callback function),
- "Grouped IEs", so that an IE payload is a nested IE structure,
- optional/mandatory/multi-occurence IEs,
- decoding unordered tags (or enforcing strict order).
Will be used for PFCP message decoding and encoding, a T16L16V protocol
which requires above features.
Upcoming patches add
- translating PDUs to plain C structs and vice versa
- TLV generator to reduce repetition a in protocol definition
- TLIV capability
Previously, the way we deal with TLVs causes a lot of code
re-implementation: the TL decoding is taken care of by the API, but for
encoding, we essentially re-implement each protocol and each encoded
message in the individual programs. This API is an improvement in that
we only once implement the TL coding (or just use osmo_t8l8v_cfg /
osmo_t16l16v_cfg), get symmetric de- and encoding of the TL, and only
need to deal with the value part of each IE.
The common pattern of
- store TL preliminarily,
- write V data and
- update L after V is complete
is conveniently done by osmo_gtlv_put_update_tl().
(2) Decoding and encoding user data:
Add osmo_gtlv_coding: describe the value part of a TLV (decode and
encode), describe a struct with its members, and get/put readily decoded
structs from/to a raw PDU, directly.
With osmo_gtlv_coding defined for a protocol's tags, we only deal with
encoded PDUs or fully decoded C structs, no TLV related
re-implementations clutter up the message handling code.
A usage example is given in gtlv_dec_enc_test. The first real use will be
the PFCP protocol in osmo-upf.git.
With osmo_gtlv_coding, there still is a lot of monkey work involved in
describing the decoded structs. A subsequent patch adds a generator for
osmo_gtlv_coding and message structs from tag value lists.
(3) TLIV support:
During code review, it was indicated that some TLV protocols that we
will likely deal with in the near future also employ an I, an instance
value of a tag. Add TLIV support.
A usage example for a manually implemented TLIV structure is found in
tests/libosmo-gtlv/gtlv_test.c.
A usage example for a generated TLIV protocol is found in
tests/libosmo-gtlv/test_tliv/.
Related: OS#5599
Related: Id72cdf94da60d4b6d09d0044c74e672c4412c15d (osmo-upf)
Change-Id: I25ab400f0c5707fdc0d8e480aca19871c2e26e71
---
M Makefile.am
M configure.ac
M include/Makefile.am
A include/osmocom/gtlv/gtlv.h
A include/osmocom/gtlv/gtlv_dec_enc.h
A include/osmocom/gtlv/gtlv_gen.h
A libosmo-gtlv.pc.in
A src/libosmo-gtlv/Makefile.am
A src/libosmo-gtlv/gtlv.c
A src/libosmo-gtlv/gtlv_dec_enc.c
A src/libosmo-gtlv/gtlv_gen.c
M tests/Makefile.am
A tests/libosmo-gtlv/Makefile.am
A tests/libosmo-gtlv/gtlv_dec_enc_test.c
A tests/libosmo-gtlv/gtlv_dec_enc_test.ok
A tests/libosmo-gtlv/gtlv_test.c
A tests/libosmo-gtlv/gtlv_test.ok
A tests/libosmo-gtlv/test_gtlv_gen/Makefile.am
A tests/libosmo-gtlv/test_gtlv_gen/gen__myproto_ies_auto.c
A tests/libosmo-gtlv/test_gtlv_gen/gtlv_gen_test.c
A tests/libosmo-gtlv/test_gtlv_gen/gtlv_gen_test.ok
A tests/libosmo-gtlv/test_gtlv_gen/myproto_ies_custom.c
A tests/libosmo-gtlv/test_gtlv_gen/myproto_ies_custom.h
A tests/libosmo-gtlv/test_tliv/Makefile.am
A tests/libosmo-gtlv/test_tliv/gen__myproto_ies_auto.c
A tests/libosmo-gtlv/test_tliv/myproto_ies_custom.c
A tests/libosmo-gtlv/test_tliv/myproto_ies_custom.h
A tests/libosmo-gtlv/test_tliv/tliv_test.c
A tests/libosmo-gtlv/test_tliv/tliv_test.ok
M tests/testsuite.at
30 files changed, 4,780 insertions(+), 2 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/86/28286/2
--
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-Attention: neels <nhofmeyr(a)sysmocom.de>
Gerrit-MessageType: newpatchset
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 5:
(2 comments)
File include/osmocom/bsc/bts.h:
https://gerrit.osmocom.org/c/osmo-bsc/+/28276/comment/65277dc1_14f22274
PS4, Line 524: chan_alloc_allow_tch_for_signalling
> cosmetic: I'd rename it to "tch_for_signalling_policy" or something like that which makes it more cl […]
Done
File src/osmo-bsc/bts_vty.c:
https://gerrit.osmocom.org/c/osmo-bsc/+/28276/comment/f1bfae62_997c3ccd
PS4, Line 568: DEFUN_ATTR
> it might be nice to add a backwards-compatibility alias for the (0|1) so old config files still pars […]
Agreed. This should be in place already, perhaps I've not gotten it correct though.
--
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: 5
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: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: dexter <pmaier(a)sysmocom.de>
Gerrit-Comment-Date: Tue, 14 Jun 2022 20:18:28 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: laforge <laforge(a)osmocom.org>
Gerrit-MessageType: comment
Attention is currently required from: iedemam, neels, pespin, 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 (#5).
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 adds the ability to completely disable signaling on TCH.
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, 43 insertions(+), 23 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-bsc refs/changes/76/28276/5
--
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: 5
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: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: dexter <pmaier(a)sysmocom.de>
Gerrit-MessageType: newpatchset
laforge has submitted this change. ( https://gerrit.osmocom.org/c/pysim/+/28277 )
Change subject: pySim-shell: set default ADM key reference
......................................................................
pySim-shell: set default ADM key reference
ETSI TS 102 221, Table 9.3 specifies 0x0A as default key reference for
ADM1. Lets make sure pySim-shell uses this key-reference if the card is
a generic UICC.
Change-Id: I8a96244269dc6619f39a5369502b15b83740ee45
---
M pySim-shell.py
1 file changed, 10 insertions(+), 0 deletions(-)
Approvals:
Jenkins Builder: Verified
laforge: Looks good to me, approved
diff --git a/pySim-shell.py b/pySim-shell.py
index f498dea..e52c4ef 100755
--- a/pySim-shell.py
+++ b/pySim-shell.py
@@ -83,16 +83,26 @@
print("Card not readable!")
return None, None
+ generic_card = False
card = card_detect("auto", scc)
if card is None:
print("Warning: Could not detect card type - assuming a generic card type...")
card = SimCard(scc)
+ generic_card = True
profile = CardProfile.pick(scc)
if profile is None:
print("Unsupported card type!")
return None, card
+ # ETSI TS 102 221, Table 9.3 specifies a default for the PIN key
+ # references, however card manufactures may still decide to pick an
+ # arbitrary key reference. In case we run on a generic card class that is
+ # detected as an UICC, we will pick the key reference that is officially
+ # specified.
+ if generic_card and isinstance(profile, CardProfileUICC):
+ card._adm_chv_num = 0x0A
+
print("Info: Card is of type: %s" % str(profile))
# FIXME: This shouln't be here, the profile should add the applications,
--
To view, visit https://gerrit.osmocom.org/c/pysim/+/28277
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: pysim
Gerrit-Branch: master
Gerrit-Change-Id: I8a96244269dc6619f39a5369502b15b83740ee45
Gerrit-Change-Number: 28277
Gerrit-PatchSet: 1
Gerrit-Owner: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-MessageType: merged
laforge has submitted this change. ( https://gerrit.osmocom.org/c/osmo-hnbgw/+/28282 )
Change subject: mgw_fsm: Change macro to not use local variables implicitly
......................................................................
mgw_fsm: Change macro to not use local variables implicitly
This is misleading for readers since it may access variables which may
be uninitialized or in a wrong state. Furthermore, we want to pass some
other variable name in a follow up patch.
This effectively allows the compiler to warn about uninitialized used of
a fi var in line 661.
Change-Id: Id694f51bb2918fd27da87b3f4a905727cd7f5de6
---
M src/osmo-hnbgw/mgw_fsm.c
1 file changed, 5 insertions(+), 5 deletions(-)
Approvals:
Jenkins Builder: Verified
laforge: Looks good to me, approved
diff --git a/src/osmo-hnbgw/mgw_fsm.c b/src/osmo-hnbgw/mgw_fsm.c
index 5dd613f..2b7484f 100644
--- a/src/osmo-hnbgw/mgw_fsm.c
+++ b/src/osmo-hnbgw/mgw_fsm.c
@@ -134,7 +134,7 @@
[MGW_ST_CRCX_MSC] = {.T = -1004 },
};
-#define mgw_fsm_state_chg(state) \
+#define mgw_fsm_state_chg(fi, state) \
osmo_tdef_fsm_inst_state_chg(fi, state, mgw_fsm_timeouts, mgw_fsm_T_defs, -1)
static void mgw_fsm_crcx_hnb_onenter(struct osmo_fsm_inst *fi, uint32_t prev_state)
@@ -206,7 +206,7 @@
return;
}
- mgw_fsm_state_chg(MGW_ST_ASSIGN);
+ mgw_fsm_state_chg(fi, MGW_ST_ASSIGN);
return;
default:
OSMO_ASSERT(false);
@@ -237,7 +237,7 @@
{
switch (event) {
case MGW_EV_RAB_ASS_RESP:
- mgw_fsm_state_chg(MGW_ST_MDCX_HNB);
+ mgw_fsm_state_chg(fi, MGW_ST_MDCX_HNB);
return;
default:
OSMO_ASSERT(false);
@@ -325,7 +325,7 @@
osmo_fsm_inst_state_chg(fi, MGW_ST_FAILURE, 0, 0);
return;
}
- mgw_fsm_state_chg(MGW_ST_CRCX_MSC);
+ mgw_fsm_state_chg(fi, MGW_ST_CRCX_MSC);
return;
default:
OSMO_ASSERT(false);
@@ -725,7 +725,7 @@
snprintf(fsm_name, sizeof(fsm_name), "mgw-fsm-%u-%u", map->rua_ctx_id, mgw_fsm_priv->rab_id);
fi = osmo_fsm_inst_alloc(&mgw_fsm, map, mgw_fsm_priv, LOGL_DEBUG, fsm_name);
map->mgw_fi = fi;
- mgw_fsm_state_chg(MGW_ST_CRCX_HNB);
+ mgw_fsm_state_chg(fi, MGW_ST_CRCX_HNB);
return 0;
error:
--
To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/28282
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-hnbgw
Gerrit-Branch: master
Gerrit-Change-Id: Id694f51bb2918fd27da87b3f4a905727cd7f5de6
Gerrit-Change-Number: 28282
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-MessageType: merged