Attention is currently required from: falconia.
fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/37558?usp=email )
Change subject: codec: add osmo_hr_sid_classify()
......................................................................
Patch Set 2:
(2 comments)
File src/codec/hr_sid_class.c:
https://gerrit.osmocom.org/c/libosmocore/+/37558/comment/9781f8e8_314e66a0
PS2, Line 35: for (; byte; byte >>= 1) {
This is absolutely fine for not so hot code paths, but if this is to be done for each byte of each RTP frame, I would recommend using a look-up table here. It can contain bit count for all 4-bit values, so then you could do:
```
return table[byte >> 4] + table[byte & 0x0f].
```
https://gerrit.osmocom.org/c/libosmocore/+/37558/comment/91be866d_6ca1e1ec
PS2, Line 144: class1_ones = sid_field_ones - class2_ones;
I am wondering if, in theory, `class2_ones` can be greater than `sid_field_ones`?
We'll end up with a huge number in this case.
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/37558?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I5f4eb65379646125b966cf182775b6e9348900bd
Gerrit-Change-Number: 37558
Gerrit-PatchSet: 2
Gerrit-Owner: falconia <falcon(a)freecalypso.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: falconia <falcon(a)freecalypso.org>
Gerrit-Comment-Date: Mon, 29 Jul 2024 07:58:30 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: pespin.
fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/37645?usp=email )
Change subject: library/GTPv1U_Templates: Mark parameters as templates
......................................................................
Patch Set 1: Code-Review-1
(4 comments)
File library/GTPv1U_Templates.ttcn:
https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/37645/comment/b0c7daf0_07e3…
PS1, Line 58: template (value)
What do we win by doing so? IMO, template parameters are useful for complex parameters, like records and unions, as you can init some/all of their fields in a template. But for parameters of simple types like OCT or INT, I don't really see the benefits of doing so. One thing I can think of is allowing to pass multiple values, like `msg_type := (MSGT_FOO, MSGT_BAR)`, but this makes no sense for a send template.
https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/37645/comment/c830ba91_7ea0…
PS1, Line 61: template (value) GTPU_IEs ies
It's fine making this one a template param, so that you can pass templates like `ts_UEchoReqPDU` directly without having to use `valueof()`. But I am not sure about the other params.
https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/37645/comment/8329c7bf_aa2c…
PS1, Line 122: valueof(ts_UEchoReqPDU
... so you turned this param into a template, but still doing unnecessary `valueof()`.
https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/37645/comment/135ff58d_7719…
PS1, Line 145: valueof(ip_addr)
... see, you expect a value here anyway, so why adding additional transformations to a template and then back to a value?
--
To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/37645?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: I278f7dbc64704c1ba2b8a75d6f540ac52b067598
Gerrit-Change-Number: 37645
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Mon, 29 Jul 2024 07:29:44 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: pespin.
fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/37625?usp=email )
Change subject: sgsn: Introduce test TC_attach_pdp_act_pmm_idle_lost_pdp_status
......................................................................
Patch Set 2: Code-Review+1
(1 comment)
File sgsn/SGSN_Tests_Iu.ttcn:
https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/37625/comment/43edb82c_a8e7…
PS2, Line 213: 1001
shouldn't we increment this (IMSI?) value in every new test?
--
To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/37625?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: I34a0dabc37ba24d0c9fb1ae2587e7ec8c1b606fa
Gerrit-Change-Number: 37625
Gerrit-PatchSet: 2
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Mon, 29 Jul 2024 07:19:13 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: pespin.
fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/37617?usp=email )
Change subject: sgsn: Introduce test TC_attach_pdp_act_pmm_idle
......................................................................
Patch Set 2:
(3 comments)
File sgsn/BSSGP_ConnHdlr.ttcn:
https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/37617/comment/16db30f7_17af…
PS2, Line 663: var RANAP_PDU ranap;
unused
https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/37617/comment/40ce0d9d_bd41…
PS2, Line 671: timer T := 5.0;
you are not starting this timer?
https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/37617/comment/93a908b8_d75b…
PS2, Line 784: var template Recovery_gtpc recovery := omit;
It's always `omit` in this altstep, so just pass `omit` to `ts_GTPC_UpdatePdpRespGGSN`?
--
To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/37617?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: Id46ccd9db11c8b792e1c071de91ef092ed1544c7
Gerrit-Change-Number: 37617
Gerrit-PatchSet: 2
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: osmith <osmith(a)sysmocom.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Mon, 29 Jul 2024 07:08:29 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment