Attention is currently required from: laforge, lynxis lazus.
daniel has posted comments on this change by lynxis lazus. ( https://gerrit.osmocom.org/c/osmo-msc/+/38107?usp=email )
Change subject: libvlr: replace direct call of paging_expired() into a callback
......................................................................
Patch Set 3: Code-Review+2
--
To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/38107?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Change-Id: I7a3d15e8f0fb51c6b32add2de5024fc4d599ecf0
Gerrit-Change-Number: 38107
Gerrit-PatchSet: 3
Gerrit-Owner: lynxis lazus <lynxis(a)fe80.eu>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: daniel <dwillmann(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: lynxis lazus <lynxis(a)fe80.eu>
Gerrit-Comment-Date: Thu, 10 Oct 2024 12:21:16 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Attention is currently required from: daniel, lynxis lazus.
fixeria has posted comments on this change by lynxis lazus. ( https://gerrit.osmocom.org/c/osmo-sgsn/+/37864?usp=email )
Change subject: Add Routing Areas
......................................................................
Patch Set 15:
(1 comment)
File tests/testsuite.at:
https://gerrit.osmocom.org/c/osmo-sgsn/+/37864/comment/21760efe_2f965942?us… :
PS15, Line 21: ignore
> I tend to use stderr for output meant for humans, which can also contain random data.
But in this specific case the stderr contains logging generated by test. It might be a good idea to match at least the `DRA` related logging here. We don't seem to be doing this here in osmo-sgsn, but we do in other projects (e.g. https://cgit.osmocom.org/osmo-bts/tree/tests/power/ms_power_loop_test.err).
--
To view, visit https://gerrit.osmocom.org/c/osmo-sgsn/+/37864?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: osmo-sgsn
Gerrit-Branch: master
Gerrit-Change-Id: I2474b19a7471a1dea3c863ddf8372b16180211aa
Gerrit-Change-Number: 37864
Gerrit-PatchSet: 15
Gerrit-Owner: lynxis lazus <lynxis(a)fe80.eu>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-CC: laforge <laforge(a)osmocom.org>
Gerrit-CC: osmith <osmith(a)sysmocom.de>
Gerrit-Attention: daniel <dwillmann(a)sysmocom.de>
Gerrit-Attention: lynxis lazus <lynxis(a)fe80.eu>
Gerrit-Comment-Date: Wed, 09 Oct 2024 22:07:21 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de>
Comment-In-Reply-To: lynxis lazus <lynxis(a)fe80.eu>
Attention is currently required from: daniel, fixeria.
lynxis lazus has posted comments on this change by lynxis lazus. ( https://gerrit.osmocom.org/c/osmo-sgsn/+/37864?usp=email )
Change subject: Add Routing Areas
......................................................................
Patch Set 15:
(2 comments)
File tests/gprs_routing_area/gprs_routing_area_test.c:
https://gerrit.osmocom.org/c/osmo-sgsn/+/37864/comment/f36e3e8c_179c3ccf?us… :
PS15, Line 329: static struct log_info_cat gprs_categories[] = {
> Do we really need all these logging categories for the test? […]
Most likely not, I copied it over from the other tests.
File tests/testsuite.at:
https://gerrit.osmocom.org/c/osmo-sgsn/+/37864/comment/835d378e_9aa6f937?us… :
PS15, Line 21: ignore
> Why not matching expected stderr too?
I tend to use stderr for output meant for humans, which can also contain random data.
--
To view, visit https://gerrit.osmocom.org/c/osmo-sgsn/+/37864?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: osmo-sgsn
Gerrit-Branch: master
Gerrit-Change-Id: I2474b19a7471a1dea3c863ddf8372b16180211aa
Gerrit-Change-Number: 37864
Gerrit-PatchSet: 15
Gerrit-Owner: lynxis lazus <lynxis(a)fe80.eu>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-CC: laforge <laforge(a)osmocom.org>
Gerrit-CC: osmith <osmith(a)sysmocom.de>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: daniel <dwillmann(a)sysmocom.de>
Gerrit-Comment-Date: Wed, 09 Oct 2024 21:55:48 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de>
Attention is currently required from: fixeria, lynxis lazus.
pespin has posted comments on this change by lynxis lazus. ( https://gerrit.osmocom.org/c/osmo-msc/+/38356?usp=email )
Change subject: vlr: use correct types for vlr_timer
......................................................................
Patch Set 1:
(1 comment)
Commit Message:
https://gerrit.osmocom.org/c/osmo-msc/+/38356/comment/1b402e94_dced8550?usp… :
PS1, Line 9: an integer
> ... […]
an integer is signed (can be negative or positive) by definition unlesss explicitly stated, I see no problem with this.
--
To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/38356?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Change-Id: Ic46ddb578962619fc08bd63645c76852e676fcc9
Gerrit-Change-Number: 38356
Gerrit-PatchSet: 1
Gerrit-Owner: lynxis lazus <lynxis(a)fe80.eu>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: lynxis lazus <lynxis(a)fe80.eu>
Gerrit-Comment-Date: Wed, 09 Oct 2024 21:38:43 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de>
Attention is currently required from: daniel, lynxis lazus.
fixeria has posted comments on this change by lynxis lazus. ( https://gerrit.osmocom.org/c/osmo-sgsn/+/37867?usp=email )
Change subject: Refactor diffing same GMM messages
......................................................................
Patch Set 15:
(3 comments)
Commit Message:
https://gerrit.osmocom.org/c/osmo-sgsn/+/37867/comment/be19f081_f61eaf54?us… :
PS15, Line 9: isn'nt
typo: isn't
File src/sgsn/gprs_gmm.c:
https://gerrit.osmocom.org/c/osmo-sgsn/+/37867/comment/885cad83_f2335cd8?us… :
PS15, Line 1432: Checks if two attach request contain the IEs and IE values
This needs to be updated.
https://gerrit.osmocom.org/c/osmo-sgsn/+/37867/comment/2fa1e281_be90b6eb?us… :
PS15, Line 1435: return 1/-1 following memcmp()
This is not true. From `man memcmp`:
```
The memcmp() function returns an integer
less than, equal to, or greater than zero ...
```
Now it's a positive or negative integer on mismatch.
--
To view, visit https://gerrit.osmocom.org/c/osmo-sgsn/+/37867?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: osmo-sgsn
Gerrit-Branch: master
Gerrit-Change-Id: Ie698d3a6894a5796663c22c8bfd12b47acda57e6
Gerrit-Change-Number: 37867
Gerrit-PatchSet: 15
Gerrit-Owner: lynxis lazus <lynxis(a)fe80.eu>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: daniel <dwillmann(a)sysmocom.de>
Gerrit-Attention: lynxis lazus <lynxis(a)fe80.eu>
Gerrit-Comment-Date: Wed, 09 Oct 2024 19:39:23 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Attention is currently required from: daniel, lynxis lazus.
fixeria has posted comments on this change by lynxis lazus. ( https://gerrit.osmocom.org/c/osmo-sgsn/+/37866?usp=email )
Change subject: Implement correct Routing Area based paging
......................................................................
Patch Set 14:
(1 comment)
File src/sgsn/sgsn_libgtp.c:
https://gerrit.osmocom.org/c/osmo-sgsn/+/37866/comment/be19b07c_18eb12f1?us… :
PS14, Line 869: rate_ctr_inc(rate_ctr_group_get_ctr(mm->ctrg, GMM_CTR_PAGING_PS));
:
Why not doing this inside of `sgsn_ra_geran_page_ra()`?
Does it make sense to increment the counter if `rc != 0`?
--
To view, visit https://gerrit.osmocom.org/c/osmo-sgsn/+/37866?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: osmo-sgsn
Gerrit-Branch: master
Gerrit-Change-Id: I181da9f656e394ccfcb8999021a5b7e13ca0419f
Gerrit-Change-Number: 37866
Gerrit-PatchSet: 14
Gerrit-Owner: lynxis lazus <lynxis(a)fe80.eu>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: daniel <dwillmann(a)sysmocom.de>
Gerrit-Attention: lynxis lazus <lynxis(a)fe80.eu>
Gerrit-Comment-Date: Wed, 09 Oct 2024 19:32:23 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Attention is currently required from: daniel, lynxis lazus.
fixeria has posted comments on this change by lynxis lazus. ( https://gerrit.osmocom.org/c/osmo-sgsn/+/37864?usp=email )
Change subject: Add Routing Areas
......................................................................
Patch Set 15:
(3 comments)
File src/sgsn/gprs_routing_area.c:
https://gerrit.osmocom.org/c/osmo-sgsn/+/37864/comment/f6d1087d_006a8b62?us… :
PS15, Line 1: /* SGSN Routing Area for 2G */
missing a copyright header
File tests/gprs_routing_area/gprs_routing_area_test.c:
https://gerrit.osmocom.org/c/osmo-sgsn/+/37864/comment/41c9264b_24312cf9?us… :
PS15, Line 329: static struct log_info_cat gprs_categories[] = {
Do we really need all these logging categories for the test?
Why you are not adding DRA here?
File tests/testsuite.at:
https://gerrit.osmocom.org/c/osmo-sgsn/+/37864/comment/cbd78f7c_56d38119?us… :
PS15, Line 21: ignore
Why not matching expected stderr too?
--
To view, visit https://gerrit.osmocom.org/c/osmo-sgsn/+/37864?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: osmo-sgsn
Gerrit-Branch: master
Gerrit-Change-Id: I2474b19a7471a1dea3c863ddf8372b16180211aa
Gerrit-Change-Number: 37864
Gerrit-PatchSet: 15
Gerrit-Owner: lynxis lazus <lynxis(a)fe80.eu>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-CC: laforge <laforge(a)osmocom.org>
Gerrit-CC: osmith <osmith(a)sysmocom.de>
Gerrit-Attention: daniel <dwillmann(a)sysmocom.de>
Gerrit-Attention: lynxis lazus <lynxis(a)fe80.eu>
Gerrit-Comment-Date: Wed, 09 Oct 2024 19:22:49 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No